Repository navigation
test(audio): stop asserting statechange order on the raw binding - #205
Merged
Merged
Conversation
The binding's statechange (a threadsafe function) and the suspend/resume/close promises (AsyncTasks) reach JS through separate libuv queues, so the handler can run after a promise settles and read a later state. The wrapper in napi.ts orders them as Chrome does; the binding never did. Check the final state instead, wait for the closed event rather than sleeping, and drop the Linux-only todo, since the same race also failed on Windows x64.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The realtime context test in
crates/audio-context-napifails intermittently, and it is blocking the 3.0.5 npm release (Windows x64).Cause
The raw binding delivers
statechangethrough a threadsafe function.suspend()/resume()/close()resolve through anAsyncTask. They reach JS through separate libuv queues, so the handler can run after the promise has settled and read a later state. That is how the test sawrunning,running,running,closedwith nosuspended.Chrome's order (event before the promise continuation) is handled in the wrapper,
packages/audio-context/napi.ts(_changeState/_syncState), and late native events are dropped there. The test called the binding directly and expected an order the binding never guaranteed. Copying the wrapper's logic into Rust would add nothing for users.Change
context.stateafter eachawait. That is reliable because the native calls block until the render thread has made the change.closedevent instead of sleeping 50 ms, then checks that at least 3 state events fired and the last one wasclosed.todois gone, since the same race also hit Windows x64.setOnstatechange's doc comment now says the callback is not ordered with the promises.Testing
On Linux x64, with all 16 cores kept busy:
todoremoved)The full audio suite passes, 11/11.